-
Notifications
You must be signed in to change notification settings - Fork 608
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial whitelist for Stargate Query #2619
Conversation
This reverts commit eb216a1.
Co-authored-by: Roman <[email protected]>
@@ -17,8 +29,83 @@ import ( | |||
var StargateWhitelist sync.Map | |||
|
|||
func init() { | |||
// cosmos-sdk queries | |||
|
|||
// auth | |||
StargateWhitelist.Store("/cosmos.auth.v1beta1.Query/Account", &authtypes.QueryAccountResponse{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish there was a way to infer these w/o hard-coding them :-/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah agreed, lets make an issue to ideate/ follow-up. I think theres some magic happening in the proto-grpc backend / RPC service, that we should be able to expose some code re-use from to eliminate this in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be possible with the chnages being done in the sdk to support deterministic queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @marbar3778, can you give me the link about the changes you said, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasmbinding/stargate_whitelist.go
Outdated
// evidence | ||
StargateWhitelist.Store("/cosmos.evidence.v1beta1.Query/Evidence", &evidencetypes.QueryEvidenceResponse{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets remove this, I don't want any backwards compatability guarantees on this one lol
Why do we not have |
@p0mvn They were added temporarily as an example, removed it for now with safety concerns, and we need more in depth review on the query! |
Also all balances generally shouldn't be used, as it can have unbounded gas. Its a common footgun to not realize this |
* Add stargate queries * Fix test * Remove initial whitelist queries * Add initial bindings, change names * Remove cosmos protos * Fix lint * Update wasm binary * Revert "Update wasm binary" This reverts commit eb216a1. * Bez's review * Add test cases * Remove logs * WIP: return json marshalled response * Add Test Case * Add changelog * Modify Changelog * Normalize -> ProtoToJson * Fix merge conflict * Add stargate querier test * Add alot more test cases * Add test case for breaking grpc querier * Update wasmbinding/query_plugin_test.go Co-authored-by: Roman <[email protected]> * code review from upstream * Add initial Stargate whitelist * Fix lint * Fix test * Update wasmbinding/stargate_whitelist.go * Update wasmbinding/stargate_whitelist.go Co-authored-by: Roman <[email protected]> Co-authored-by: Dev Ojha <[email protected]> (cherry picked from commit 5380f62) # Conflicts: # wasmbinding/query_plugin_test.go # wasmbinding/stargate_whitelist.go
Closes: #2433
What is the purpose of the change
This PR adds initial list of queries to whitelist for Stargate query.
Queries using iterators to iterate through kv store is safe, while a query returning a non sorted array, or a query using randomness is considered unsafe.
Brief Changelog